Add codespell config (within pyproject.toml), action and fix the typos it finds (and fixes) #3098
Conversation
yarikoptic
force-pushed
the
enh-codespell
branch
2 times, most recently
from
a9be623
to
bebd8bc
Compare
|
Merging this PR will trigger the following deployment actions. Support and Staging deployments
Production deployments
|
yarikoptic
force-pushed
the
enh-codespell
branch
from
093fd06
to
fe202e5
Compare
|
I have removed a dedicated github action since pre-commit.ci does the checks anyways. Note that pre-commit.ci does it also on .github/ files which seems to be ignored by default if you just run |
|
before I resolve the conflicts -- please advise on either such PR would be reviewed/considered. |
|
Hi @yarikoptic! Thank you for this PR. I have opened an issue to prompt the engineering team to spend some time considering this and we can hopefully get back to you soon! |
=== Do not change lines below ===
{
"chain": [],
"cmd": "codespell -w -i 3 -C 2",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
For some reason codespell by default ignores .github/ but then
pre-commit is triggering check on those files anyways
=== Do not change lines below ===
{
"chain": [],
"cmd": "codespell -w .github/workflows/ensure-uptime-checks.yaml .github/workflows/deploy-hubs.yaml",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
=== Do not change lines below ===
{
"chain": [],
"cmd": "codespell -w || :",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
=== Do not change lines below ===
{
"chain": [],
"cmd": "codespell -w -i 3 -C 2",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
yarikoptic
force-pushed
the
enh-codespell
branch
from
fe202e5
to
28d4e3d
Compare
|
Hi @sgibson91 - what was the decision? I have now refreshed the PR but I think it would not be worthwhile the effort if there is no further interest from 2i2c. |
There was a problem hiding this comment.
First, I want to apologize for the long long wait here, @yarikoptic! We've been a little swamped, and I must've consumed too much content about GPT tools causing issues in open source projects because somehow I just assumed this was an automated PR - and did not actually follow through to check. I apologize for not doing more due diligence here.
I had also assumed that codespell was a spell checker, and we'll need to maintain a list of 'exempt' words. Turns out this is actually not the case! From https://github.com/codespell-project/codespell,
It's designed primarily for checking misspelled words in source code (backslash escapes are skipped), but it can be used with other files as well. It does not check for word membership in a complete dictionary, but instead looks for a set of common misspellings. Therefore it should catch errors like "adn", but it will not catch "adnasdfasdf". This also means it shouldn't generate false-positives when you use a niche term it doesn't know about.
So it looks like it will primarily fix the common ways specific words are misspelled, rather than a general dictionary. Often @sgibson91 catches these errors in the PRs I make, for example. So I can see how this is generally very useful!
I also don't think there's any security considerations here that are harder than regular security considerations for pre-commit anyway.
So I think this is good to go. Unless there are objections from anyone, I'll merge this next Wednesday (Jan 31)
Thank you for your patience as we worked through this, @yarikoptic! In general PRs modifying actual config from folks in the community those hubs are serving get looked at much faster than this :)
I just want to clarify that you might still need to do that since by default codespell e.g. uses a dictionary with rare words - ie where typos are still valid but rare words. Or some times it is some sentence not in English or some odd variable name etc. Hence there is that config where I already added a few skips on files and 1 exempt word: https://github.com/2i2c-org/infrastructure/pull/3098/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R15 . Overall -- it would still be worth to eyeball if there is no some false positive or an undesired code change in this PR diff. |
| @@ -297,7 +297,7 @@ grafana: | |||
| # prometheus and grafana. | |||
| # | |||
| # Grafana's memory use seems to increase over time but seems reasonable to | |||
| # stay below 200Mi for years to come. Grafana's CPU use seems miniscule with | |||
| # stay below 200Mi for years to come. Grafana's CPU use seems minuscule with | |||
There was a problem hiding this comment.
e.g. here is a case of a "rare" word -- AFAIK minuscule is a more proper/older version, but recently miniscule also got to be used
There was a problem hiding this comment.
right. this is also why i like this being pre-commit, because it just fixes it and there's no discussion that needs to be had. pre-commit imperialism ftw ;)
There was a problem hiding this comment.
just to make sure it was not missed since I failed to mention in PR description -- there is pre-commit config provided too in this PR: https://github.com/2i2c-org/infrastructure/pull/3098/files#diff-63a9c44a44acf85fea213a857769990937107cf072831e1a26808cfde9d096b9
|
@yarikoptic yep that makes sense - I'm ok with that, since the list of exceptions is much fewer (aks I guess is a common misspelling of ask). I did eyeball this change and it looks ok. What I didn't want was to end up having to maintain something like https://github.com/jupyterhub/jupyterhub/blob/041acbc0bf186f2810816d16be4c65af93d84795/docs/source/spelling_wordlist.txt#L162 which is much bigger :) |
|
It's wednesday, mergey mergey. Thank you for your patience, @yarikoptic! |
|
🎉🎉🎉🎉 Monitor the deployment of the hubs here 👉 https://github.com/2i2c-org/infrastructure/actions/runs/7736874709 |
No description provided.